Skip to content

Modernize workflows - #136

Closed
mmorel-35 wants to merge 1 commit into
SkyAPM:mainfrom
mmorel-35:runners
Closed

Modernize workflows#136
mmorel-35 wants to merge 1 commit into
SkyAPM:mainfrom
mmorel-35:runners

Conversation

@mmorel-35

@mmorel-35 mmorel-35 commented Sep 20, 2025

Copy link
Copy Markdown
Contributor

Description

Ubuntu-20.04 is not supported by github anymore.
actions/checkout@v3 is also not documented in the README

Also uses actions/checkouts instead of git clones, will reduce the space taken as the reference is precised with only one commit being checked out.

I also used stepsecurity to define permissions appropriately with commit sha

See

@mmorel-35 mmorel-35 mentioned this pull request Sep 20, 2025
@mmorel-35 mmorel-35 changed the title Upgrade workflow runners to Ubuntu 22.04 Modernize workflows Sep 21, 2025
wbpcode
wbpcode previously approved these changes Sep 23, 2025

@wbpcode wbpcode left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks.

@wbpcode

wbpcode commented Sep 23, 2025

Copy link
Copy Markdown
Contributor

ci of c++11 is not supported. But I think maybe you can disable it first. And create an issue for that. We can see whether we can fix it or just abandon c++11 in our new version.

@mmorel-35

Copy link
Copy Markdown
Contributor Author

I believe it is better to drop support for version before c++17 as long as dependencies like protobuf require c++17, see
https://github.com/protocolbuffers/protobuf/blob/main/.bazelrc

@mmorel-35 mmorel-35 mentioned this pull request Sep 23, 2025
@wbpcode

wbpcode commented Sep 23, 2025

Copy link
Copy Markdown
Contributor

I am OK to that. We won't add too much new features to cpp2sky. So, I think it's fine to support only c++17 in our new version. who won't the old version support still could use our old version.

@mmorel-35

Copy link
Copy Markdown
Contributor Author

I have removed '-Werror' so it doesn't fail on warnings.

@mmorel-35

mmorel-35 commented Sep 24, 2025

Copy link
Copy Markdown
Contributor Author

Following https://stackoverflow.com/a/39194946 recommendation

@wbpcode

wbpcode commented Oct 10, 2025

Copy link
Copy Markdown
Contributor

@mmorel-35 sorry for the delay. There is no pr-reminder there Orz.

Signed-off-by: Matthieu MOREL <matthieu.morel35@gmail.com>
@mmorel-35

Copy link
Copy Markdown
Contributor Author

@wbpcode ,
Can you try again ? If this is not working my next change is to update bazelversion as to my understanding, in v4 of bazel all warnings are threated are errors but the disable of those three shall do the job

@wbpcode

wbpcode commented Oct 12, 2025

Copy link
Copy Markdown
Contributor

CI still not happy.

warning: unknown warning option '-Wno-array-parameter'; did you mean '-Wno-unused-parameter'? [-Wunknown-warning-option]

@wbpcode wbpcode closed this in #135 Oct 25, 2025
@mmorel-35
mmorel-35 deleted the runners branch October 25, 2025 14:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants